Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: deprecate _toBuf #22501

Closed
wants to merge 3 commits into from
Closed

Conversation

tniessen
Copy link
Member

This function is almost trivial and was not supposed to be visible outside of Node.js core.

I am not entirely happy with the wording in the deprecation documentation, suggestions are welcome.

Fixes: #22425

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. tls Issues and PRs related to the tls subsystem. labels Aug 24, 2018
@tniessen tniessen added the semver-major PRs that contain breaking changes and should be released in the next major version. label Aug 24, 2018
Type: Runtime

The `crypto._toBuf()` function was not designed to be used by modules outside
of Node.js core and will become unavailable in the future.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change "will become unavailable" with "will be removed"?

@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 24, 2018
@addaleax
Copy link
Member

@Trott
Copy link
Member

Trott commented Aug 25, 2018

@nodejs/security-wg

@tniessen tniessen added the deprecations Issues and PRs related to deprecations. label Aug 25, 2018
@tniessen
Copy link
Member Author

Seems like CI was interrupted, resumed in: https://ci.nodejs.org/job/node-test-pull-request/16761/

tniessen added a commit that referenced this pull request Aug 27, 2018
PR-URL: #22501
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
tniessen added a commit that referenced this pull request Aug 27, 2018
PR-URL: #22501
Fixes: #22425
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
@tniessen
Copy link
Member Author

tniessen commented Aug 27, 2018

Landed in fa3d6be, 50aa85d with deprecation code DEP0114. Thanks for reviewing, everyone.

@tniessen tniessen closed this Aug 27, 2018
tniessen added a commit to tniessen/node that referenced this pull request Aug 27, 2018
tniessen added a commit that referenced this pull request Aug 30, 2018
PR-URL: #22551
Refs: #22501
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@tniessen tniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. deprecations Issues and PRs related to deprecations. semver-major PRs that contain breaking changes and should be released in the next major version. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should _toBuf be renamed and documented?
10 participants